Add support for MiniO storage backend#667
Conversation
|
@loic-sharma Can you please add this? I like BaGet but would like to use MinIO as storage backend in my kubernetes cluster instead of nfs. |
| "ServiceUrl": "", | ||
| "UseHttp": false, | ||
| "ForcePathStyle": false |
There was a problem hiding this comment.
It'd be less confusing to folks if we separate the AWS S3 and MinIO samples:
| "ServiceUrl": "", | |
| "UseHttp": false, | |
| "ForcePathStyle": false |
|
Hello @alfeg, thank you for the contribution! Your changes look good. I suggested some minor tweaks but I've never used MinIO. Could you let me know what you think? @viceice and @DTeuchert Feel free to jump in too if you have comments! |
|
|
||
| public bool ForcePathStyle { get; set; } = false; | ||
|
|
||
| public string ServiceUrl { get; set; } |
There was a problem hiding this comment.
Could we add the data annotation attributes that @DTeuchert had used in #272?
- [Required]
+ [RequiredIf(nameof(ServiceUrl), null)]
public string Region { get; set; }
+ [RequiredIf(nameof(Region), null)]
public string ServiceUrl { get; set; }
There was a problem hiding this comment.
Yes in my understanding we need a service url for a private / on premise solutions or a region.
| var config = new AmazonS3Config | ||
| { | ||
| RegionEndpoint = RegionEndpoint.GetBySystemName(options.Region) | ||
| RegionEndpoint = RegionEndpoint.GetBySystemName(options.Region), |
There was a problem hiding this comment.
From my understanding the region can be set using the service URL. If so should we follow @DTeuchert's pattern in #272:
| RegionEndpoint = RegionEndpoint.GetBySystemName(options.Region), | |
| RegionEndpoint = (options.Region != null) | |
| ? RegionEndpoint.GetBySystemName(options.Region) | |
| : null, | |
| ServiceURL = options.ServiceUrl |
This will let us delete lines 47-50 below:
- if (!string.IsNullOrWhiteSpace(options.ServiceUrl))
- {
- config.ServiceURL = options.ServiceUrl;
- }There was a problem hiding this comment.
I think the AWS SDK requires a region independent from service url, but im not fully sure.
Hello, I'm OK with all proposed changes, but cannot test them in any way for next two weeks. I'm on vacation without laptop and with poor internet :) |
|
On Monday I can test this request with a ceph cluster as s3 backend. |
|
are there any plans on following up on this ? |
|
you should check |
|
@loic-sharma Any plans on this? |
|
No. I'd consider checking out BaGetter instead, it is an active fork of BaGet. |
that is also not having support of MinIO. |
6 participants
This PR is yet another attempt to address #621, #551, and #271
Added additional settings to
S3StorageOptions:ForcePathStyle = falsesetting to true will make AWSSDK.net compatible with MiniO installationsServiceUrl- ability to override S3 service locationUseHttp- ability to use HTTP protocol forServiceUrlAll those settings are real settings from actual
AmazonS3Configfrom official AWSSDK libraryThere is already another PR's to address those issues: #644 and #272
My PR is much less intrusive, only adding ability to pass settings that are already exists in AwsSdk library